-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tatanka/client: separate network parts from mesh protocol parts #3176
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely makes sense to separate like this.
} | ||
|
||
// iterate iterates a table or index. | ||
func (db *DB) iterate(keyPfix keyPrefix, table *Table, io iteratorOpts, isIndex bool, prefixI IndexBucket, f func(*Iter) error, iterOpts ...IterationOption) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (db *DB) iterate(keyPfix keyPrefix, table *Table, io iteratorOpts, isIndex bool, prefixI IndexBucket, f func(*Iter) error, iterOpts ...IterationOption) error { | |
func (db *DB) iterate(keyPfix keyPrefix, table *Table, defaultOpts iteratorOpts, isIndex bool, prefixI IndexBucket, f func(*Iter) error, iterOpts ...IterationOption) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a value, so it's copied on the way in. We work on the copy internally, so it wouldn't make sense to name it "default" any more.
// UseDefaultIterationOptions sets default options for Iterate. | ||
func (t *Table) UseDefaultIterationOptions(optss ...IterationOption) { | ||
for i := range optss { | ||
optss[i](&t.defaultIterationOptions) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused.
if err != nil { | ||
return err | ||
} | ||
enc, err := p.encryptRSA(b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is already existing code, but what's the reason for having a separate RSA key pair in addition to the ECDSA one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also confused about the signing patterns.
If we just use ws/wss we wouldn't need to sign all messages to prove who sent them, right? wss would encrypt.
If the purpose is to prove to a third party that this message was sent, then we use the signature from the private key of the secp256k1.PublicKey that is unique for every user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RSA keypair is for encryption/decryption of p2p messages. This is end-to-end encryption. Server can't read the messages.
If we just use ws/wss we wouldn't need to sign all messages to prove who sent them, right?
Right. For WSS connections, only the initial message needs to be signed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option worth considering instead of RSA is AES for symmetric encryption. Both peers would use the same key, derived via ECDH using the user’s private key and the counterparty’s public key (e.g., existing secp256k1 keys from DCRDEX identities). This skips a separate key exchange since we’re leveraging keys we already have, and AES is way faster than RSA, microseconds versus milliseconds per message. Here's how to derive an AES key:
func deriveAESKey(privKey *btcec.PrivateKey, peerPubKey *btcec.PublicKey) ([]byte, error) {
// ECDH: Compute shared secret
sharedX, _ := privKey.ECDH(peerPubKey)
sharedSecret := sharedX.Bytes() // x-coordinate (32 bytes)
// HKDF: Derive AES-256 key
hkdf := hkdf.New(sha256.New, sharedSecret, nil, nil)
aesKey := make([]byte, 32) // 256 bits
_, err := hkdf.Read(aesKey)
if err != nil {
return nil, fmt.Errorf("failed to derive AES key: %v", err)
}
return aesKey, nil
}
func (m *Mesh) ConnectPeer(peerID tanka.PeerID) error { | ||
return m.conn.ConnectPeer(peerID) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only exposed for testing? Looks like right now you connect to a new peer other than the entry node when a message arrives from that peer. When connecting to the entry node, the entry node could share all of its peers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh huh. I'm not certain yet how the server node connection scheme is going to shake out. Will we connect to all the tatanka nodes that we can, or just one at a time and use the others as backups should ours go offline? When we broadcast a message, we should only send it to one, right? How about p2p messaging? Is there some kind of path optimization to be done?
And yes, this is only exposed for testing purposes for right now.
type TatankaSelectionMode string | ||
|
||
const ( | ||
SelectionModeEntryNode TatankaSelectionMode = "EntryNode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With what kind of request would you only want to message the entry node?
|
||
// Config is the configuration for the MeshConn. | ||
type Config struct { | ||
EntryNode *TatankaCredentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only one EntryNode
rather than a list?
tatanka/client/conn/conn.go
Outdated
if err := cm.Connect(ctx); err != nil { | ||
c.log.Errorf("error connecting to tatanka node %s at %s: %v. will keep trying to connect", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is an error that would be returned from the websocket the ctx is canceled and this won't keep trying I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. We'll have to look into reconnect handling too.
tatanka/client/conn/conn.go
Outdated
for peerID, tt := range c.tatankaNodes { | ||
if tt.cm.On() && peerID != skipID { | ||
tts = append(tts, tt) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can return earlier if Any.
if err != nil { | ||
return err | ||
} | ||
enc, err := p.encryptRSA(b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also confused about the signing patterns.
If we just use ws/wss we wouldn't need to sign all messages to prove who sent them, right? wss would encrypt.
If the purpose is to prove to a third party that this message was sent, then we use the signature from the private key of the secp256k1.PublicKey that is unique for every user.
tatanka/client/conn/conn.go
Outdated
c.handlers.HandleTatankaNotification(tatankaID, msg) | ||
return nil | ||
default: | ||
c.log.Errorf("tatanka node %s send a message with an unhandleable type %d", msg.Type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.log.Errorf("tatanka node %s send a message with an unhandleable type %d", msg.Type) | |
c.log.Errorf("tatanka node %s send a message with an unhandleable type %d", tt.peerID, msg.Type) |
I’ve updated the encryption to use AES instead of RSA in this diff: 03c89e7 It retains the connection request to establish an ephemeral key pair for each peer connection. This ensures that if a permanent private key leaks, past and future communications remain secure due to the ephemeral keys. I suggest adding a typed |
Separate the networking logic from the mesh protocol logic by splitting the old TankaClient into two parts. MeshConn handles connections to both tatanka nodes and peers. Mesh handles the client mesh protocol. Mesh has its own database. That is where things like order info and bonds will be stored.
Separate the networking logic from the mesh protocol logic by splitting the old
TankaClient
into two parts.MeshConn
handles connections to both tatanka nodes and peers.Mesh
handles the client mesh protocol.Mesh
is somewhat similar tocore.dexConnection
in its purpose.Here's a taste of the separated APIs.
Mesh
has its own database. That is where things like order info and bonds will be stored. I've implemented a bonds table to demonstrate.